Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

New canvas bridge implementation #1550

Merged
merged 8 commits into from
Jan 13, 2023
Merged

New canvas bridge implementation #1550

merged 8 commits into from
Jan 13, 2023

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Jan 12, 2023 •
edited
Loading

Trying to set up a system where the editor can run expressions inside of the canvas. The goal of which is to improve debuggability of bindings. Running the watch expressions in the actual execution engine allows us to attach information about the page state it's depending on.

This PR aims to simplify the bridge that communicates between the editor and canvas by just having a single instance in the module scope of the page. It's always there, so it can be reached immediately on iframe onload. It also make the bridge more expressive and allow two-way communication by separating events and commands both in the direction of the editor and the direction of the canvas.

The init workflow is:

  • canvas sets up bridge on the window during page load (not async)
    • Some of the canvas initialization, rendering and setup is asynchronous in nature, when ready it will emit a ready event, it will also make ready state available synchronously through isReady().
  • editor waits for the onload of the iframe containing the canvas
  • editor accesses the bridge object from the contentWindow of the iframe
  • editor inspects canvas ready state synchrounously and waits for the ready event if not.
  • canvas is initialized, the bridge is usable in the editor.

We keep the bridge a stable object during the full lifetime of a page. For this I introduced a new primitive Commands.

@Janpot Janpot added the core Infrastructure work going on behind the scenes label Jan 12, 2023
@oliviertassinari oliviertassinari requested a deployment to bridge - toolpad-db PR #1550 January 12, 2023 18:29 — with Render Abandoned
@oliviertassinari oliviertassinari temporarily deployed to bridge - toolpad PR #1550 January 12, 2023 18:29 — with Render Destroyed
@oliviertassinari oliviertassinari temporarily deployed to bridge - toolpad PR #1550 January 13, 2023 08:24 — with Render Destroyed
@Janpot Janpot marked this pull request as ready for review January 13, 2023 08:48
@Janpot Janpot changed the title New bridge implementation New canvas bridge implementation Jan 13, 2023
@oliviertassinari oliviertassinari temporarily deployed to bridge - toolpad PR #1550 January 13, 2023 13:42 — with Render Destroyed
@oliviertassinari oliviertassinari temporarily deployed to bridge - toolpad PR #1550 January 13, 2023 13:44 — with Render Destroyed
@Janpot
Copy link
Member Author

Janpot commented Jan 13, 2023 •
edited
Loading

@bytasv In this PR I'm also reducing the amount of flicker when loading/switching pages. The simplified init logic makes this a lot more straightforward. There are now only two loading states:

  1. Loading state in the editor, which starts as soon as navigation in the iframe starts, and stops when the Toolpad app signals it's ready to take over. It shows as a spinner on the default background
  2. Initial loading state of the Toolpad app, when it started react rendering, but when potentially it's still loading components asynchronously. Over time we should aim at making improvements to build/init logic which can fix this. This currently shows as a liner progress and also exists outside of the editor, on deployed applications.

@bytasv
Copy link
Contributor

bytasv commented Jan 13, 2023

Great job, I think the experience is much better now. One thing that I noticed now - we use white background when loading, but the canvas is gray, which creates a bigger "flickering" effect. Maybe it would be worth to consider using same gray for loading screen too so there is "less change".
Alternatively I'm not sure how more difficult it is, but if we could add loading overlay on top of existing page before new page is loaded and then if we could do a quick fade out transition for that the change of page would be much smoother, but obviously I'm just dreaming here :D

@oliviertassinari oliviertassinari temporarily deployed to bridge - toolpad PR #1550 January 13, 2023 14:56 — with Render Destroyed
@Janpot
Copy link
Member Author

Janpot commented Jan 13, 2023

we use white background when loading, but the canvas is gray

  • The white here is the theme background color of the editor, it will be dark blue in dark mode
  • The grey here is the theme background color of the Toolpad app, it will be black when the app is in dark mode

It feels sensible to me to keep the themes separate for now. I'd like to concentrate rather of getting rid of the second loading state.

but if we could add loading overlay on top of existing page before new page is loaded

That's exactly how this works now, there is no other way to overcome the flicker in iframe loading between when navigation has started and the onload event.

but obviously I'm just dreaming here

Not at all, the MUI Fade component makes this trivial to implement.

@Janpot Janpot merged commit 7c113d0 into master Jan 13, 2023
@Janpot Janpot deleted the bridge branch January 13, 2023 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bytasv bytasv bytasv approved these changes

@apedroferreira apedroferreira Awaiting requested review from apedroferreira

Assignees
No one assigned
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Janpot @bytasv @oliviertassinari